Feature: Extend for mixins #2096
Conversation
|
Thanks for the PR! This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged. |
inputfiles/patches/childnode.kdl
Outdated
| @@ -0,0 +1 @@ | |||
| interface-mixin ChildNode extends="Node" | |||
There was a problem hiding this comment.
Also I don't think it's worth having a separate file for a single line change, but then I'm not sure what would be the right name. Maybe misc.kdl is fine and we can put random simple changes here.
There was a problem hiding this comment.
I think extensions is a good name, because there are a bunch of things that we extend
There was a problem hiding this comment.
And please cover ParentNode too, and some comment like:
// ChildNode and ParentNode are actually defined as mixins, but because of their names they have historically been used as a sub-interface of Node.
... with some line wrapping.
There was a problem hiding this comment.
I think extensions is a good name, because there are a bunch of things that we extend
Actually I change my mind here, covering ChildNode/ParentNode with some context is worth a separate file
There was a problem hiding this comment.
Now child-parent-node.kdl or such? With that LGTM.
src/build/patches.ts
Outdated
| const result: any = { | ||
| name, | ||
| events: { event }, | ||
| properties: { property }, | ||
| } as DeepPartial<Interface>; | ||
| }; |
There was a problem hiding this comment.
| const result: any = { | |
| name, | |
| events: { event }, | |
| properties: { property }, | |
| } as DeepPartial<Interface>; | |
| }; | |
| const result = { | |
| name, | |
| events: { event }, | |
| properties: { property }, | |
| } as DeepPartial<Interface>; |
It's not clear to me why const result: DeepPartial<Interface> doesn't work (the error is confusingly complex), but this should be ok.
…fining return type and casting 'extends' property
|
LGTM |
|
There was an issue merging, maybe try again saschanaz. Details |
|
LGTM |
|
Merging because @saschanaz is a code-owner of all the changes - thanks! |
3 participants
related #2053